Skip to content

Malformed data while Content Type is not specified in multipart forms #2934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
4 tasks done
Usioumeo opened this issue Apr 22, 2025 · 1 comment
Open
4 tasks done
Labels
triage A bug report being investigated

Comments

@Usioumeo
Copy link

Rocket Version

0.5.1

Operating System

Arch Linux

Rust Toolchain Version

rustup 1.28.1 (2025-03-05)

What happened?

We recently migrated our association code from rocket 0.4 to rocket 0.5.1. First there was no support for multipart data, and so we relied on rocket_multipart_form_data - crate, then we decided to use the TempFile data guard provided by rocket. We have a generic form upload for image, and for many format we support there is no valid ContentType, and because of that we decided to not specify it (given that it should be optional by our knowledge, and should not modify the parsing in any way).

But then we started getting malformed data out of our system.
I recreated a minimal reproducible to illustrate the issue.

Test Case

use reqwest::multipart;
use rocket::{FromForm, form::Form, fs::TempFile, launch, post, routes};

#[derive(FromForm)]
struct DummyForm<'a> {
    image: TempFile<'a>,
}

#[post("/", data = "<data>")]
async fn small_post(mut data: Form<DummyForm<'_>>) -> &'static str {
    //save file to a known location in order to read its content
    data.image.persist_to("/tmp/recv.png").await.unwrap();
    let recv = std::fs::read("/tmp/recv.png").unwrap();
    //generate same data
    let bytes: Vec<u8> = (0..140).map(|x| (x % 256) as u8).collect();
    //check if the data are the same
    assert_eq!(bytes, recv);
    "ok"
}

#[launch]
async fn rocket() -> _ {
    // spawn a task to send a request
    rocket::tokio::spawn(async {
        // wait for rocket startup
        rocket::tokio::time::sleep(rocket::tokio::time::Duration::from_secs(1)).await;

        //generate bytes to send
        let bytes: Vec<u8> = (0..140).map(|x| (x % 256) as u8).collect();
        //build form ()
        let image = multipart::Part::bytes(bytes)
            //UNCOMMENT IF WANT TO SET CONTENT TYPE
            //.headers(HeaderMap::from_iter(vec![(CONTENT_TYPE, HeaderValue::from_static("image/png")),]))
            ;
        let form = multipart::Form::new().part("image", image);

        let client = reqwest::Client::builder().build().unwrap();
        let resp = client
            .post("http://localhost:8000/")
            .multipart(form)
            .send()
            .await
            .unwrap();
        println!("{:?}", resp);
    });

    rocket::build().mount("/", routes![small_post])
}

Log Output

ROCKET_LOG_LEVEL=debug cargo run                                                        ✔  7s  
   Compiling stupid v0.1.0 (/home/alessio/Documents/Mindshub/insignio/insigno_backend/stupid)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.28s
     Running `target/debug/stupid`
-- configuration trace information --
   >> "address" parameter source: rocket::Config::default()
   >> "port" parameter source: rocket::Config::default()
   >> "workers" parameter source: rocket::Config::default()
   >> "max_blocking" parameter source: rocket::Config::default()
   >> "keep_alive" parameter source: rocket::Config::default()
   >> "ident" parameter source: rocket::Config::default()
   >> "ip_header" parameter source: rocket::Config::default()
   >> "limits" parameter source: TOML file (../Rocket.toml)
   >> "temp_dir" parameter source: rocket::Config::default()
   >> "log_level" parameter source: `ROCKET_` environment variable(s)
   >> "shutdown" parameter source: rocket::Config::default()
   >> "cli_colors" parameter source: rocket::Config::default()
🔧 Configured for debug.
   >> address: 127.0.0.1
   >> port: 8000
   >> workers: 16
   >> max blocking threads: 512
   >> ident: Rocket
   >> IP header: X-Real-IP
   >> limits: bytes = 8KiB, data-form = 64MB, file = 64MB, form = 128kB, json = 1MiB, msgpack = 1MiB, string = 8KiB
   >> temp dir: /tmp
   >> http/2: true
   >> keep-alive: 5s
   >> tls: disabled
   >> shutdown: ctrlc = true, force = true, signals = [SIGTERM], grace = 2s, mercy = 3s
   >> log level: debug
   >> cli colors: true
📬 Routes:
   >> (small_post) POST /
📡 Fairings:
   >> Shield (liftoff, response, singleton)
🛡️ Shield:
   >> X-Frame-Options: SAMEORIGIN
   >> Permissions-Policy: interest-cohort=()
   >> X-Content-Type-Options: nosniff
🚀 Rocket has launched from http://127.0.0.1:8000

--> /home/alessio/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/reqwest-0.12.15/src/connect.rs:625
        starting new connection: http://localhost:8000/

--> /home/alessio/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rocket-0.5.1/src/server.rs:77
        received request: Request {
    method: POST,
    uri: /,
    version: HTTP/1.1,
    headers: {
        "content-type": "multipart/form-data; boundary=d21ec4330d930b93-195c0b0274147312-0682dac8e0cc00cd-74d04b62a85a3e59",
        "content-length": "334",
        "accept": "*/*",
        "host": "localhost:8000",
    },
    body: Body(
        Streaming,
    ),
}
POST / multipart/form-data:
   >> Matched: (small_post) POST /
   >> multipart field: Field { state: Mutex { data: MultipartState { buffer: StreamBuffer, boundary: "d21ec4330d930b93-195c0b0274147312-0682dac8e0cc00cd-74d04b62a85a3e59", stage: ReadingFieldData, next_field_idx: 1, curr_field_name: Some("image"), curr_field_size_limit: 64000000, curr_field_size_counter: 0, constraints: Constraints { size_limit: SizeLimit { whole_stream: 64000000, per_field: 64000000, field_map: {} }, allowed_fields: None } }}, done: false, headers: {"content-disposition": "form-data; name=\"image\""}, content_disposition: ContentDisposition { field_name: Some("image"), file_name: None }, content_type: None, idx: 0 }

thread 'rocket-worker-thread' panicked at src/main.rs:17:5:
assertion `left == right` failed
  left: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139]
 right: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 239, 191, 189, 239, 191, 189, 239, 191, 189, 239, 191, 189, 239, 191, 189, 239, 191, 189, 239, 191, 189, 239, 191, 189, 239, 191, 189, 239, 191, 189, 239, 191, 189, 239, 191, 189]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
   >> Handler small_post panicked.
   >> This is an application bug.
   >> A panic in Rust must be treated as an exceptional event.
   >> Panicking is not a suitable error handling mechanism.
   >> Unwinding, the result of a panic, is an expensive operation.
   >> Panics will degrade application performance.
   >> Instead of panicking, return `Option` and/or `Result`.
   >> Values of either type can be returned directly from handlers.
   >> A panic is treated as an internal server error.
   >> Outcome: Error(500 Internal Server Error)
   >> No 500 catcher registered. Using Rocket default.

--> /home/alessio/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rocket-0.5.1/src/server.rs:166
        sending response: Response {
    status: 500,
    version: HTTP/1.1,
    headers: {
        "content-type": "text/html; charset=utf-8",
        "server": "Rocket",
        "x-frame-options": "SAMEORIGIN",
        "permissions-policy": "interest-cohort=()",
        "x-content-type-options": "nosniff",
        "content-length": "488",
    },
    body: Body(
        Streaming,
    ),
}
   >> Response succeeded.
Response { url: "http://localhost:8000/", status: 500, headers: {"content-type": "text/html; charset=utf-8", "server": "Rocket", "x-frame-options": "SAMEORIGIN", "permissions-policy": "interest-cohort=()", "x-content-type-options": "nosniff", "content-length": "488", "date": "Tue, 22 Apr 2025 14:04:40 GMT"} }

Additional Context

I haven't used the rocket local module because I do not know how to recreate the problem.
For the same reason I included the async module so that it is easily testable.

System Checks

  • My bug report relates to functionality.
  • I have tested against the latest Rocket release or a recent git commit.
  • I have tested against the latest stable rustc toolchain.
  • I was unable to find this issue previously reported.
@Usioumeo Usioumeo added the triage A bug report being investigated label Apr 22, 2025
@Usioumeo
Copy link
Author

Usioumeo commented May 30, 2025

I improved the minimal example, so it is easier to follow and debug:

use rocket::{FromForm, form::Form, fs::TempFile, launch, post, routes};

#[derive(FromForm)]
struct DummyForm<'a> {
    image: TempFile<'a>,
}

#[post("/", data = "<data>")]
async fn small_post(mut data: Form<DummyForm<'_>>) -> Vec<u8> {
    //save file to a known location in order to read its content
    data.image.persist_to("/tmp/recv.png").await.unwrap();
    let recv = std::fs::read("/tmp/recv.png").unwrap();
    println!("Received {:?} bytes", recv);
    recv
}

#[launch]
fn rocket() -> _ {
    rocket::build().mount("/", routes![small_post])
}

#[cfg(test)]
mod test {
    use super::rocket;
    use rocket::http::ContentType;
    use rocket::http::Status;
    use rocket::local::blocking::Client;

    #[test]
    fn hello_world() {
        let client = Client::tracked(rocket()).expect("valid rocket instance");
        // Manually build a multipart body
        let boundary = "boundary123";
        let str: Vec<u8> = (0..256).map(|x| (x as u8)).collect();

        let mut body = Vec::new();
        body.extend_from_slice(
            format!(
                "--{boundary}\r\n\
    Content-Disposition: form-data; name=\"image\"; filename=\"test.png\"\r\n"
            )
            .as_bytes(),
        );
        //UNCOMMENT ME IN ORDER TO USE THE CORRECT IMPLEMENTATION
        //body.extend_from_slice("Content-Type: image/png\r\n".as_bytes());
        body.extend_from_slice("\r\n".as_bytes());
        body.extend_from_slice(&str);
        body.extend_from_slice(format!("\r\n--{boundary}--\r\n").as_bytes());
        println!("Body: {:?}", &body);
        let response = client
            .post("/")
            .header(
                ContentType::new("multipart", "form-data").with_params([("boundary", boundary)]),
            )
            .body(body)
            .dispatch();

        assert_eq!(response.status(), Status::Ok);
        assert_eq!(response.into_bytes().unwrap(), str);
        
    }
}

So what appens is that when content type is not specified, the form is considered a "value" and not a "data", this means that non standard ascii get's escaped (adding data). In theory this snouldn't be the correct implementation as stated by RFC_9110. So it should suffice to set it to data without any other problem right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage A bug report being investigated
Projects
None yet
Development

No branches or pull requests

1 participant